Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/#866 matchmaker timeout #867

Merged

Conversation

Askaholic
Copy link
Collaborator

  • Add game_id to match_cancelled message so the client can choose to ignore it if the message doesn't apply to the current game
  • Prevent start_game from stalling on hung connections. The timeouts should always be followed now.

@Sheikah any preference on what to call the game_id field? I think game_id makes sense but in game_info it's called uid.

Mitigates some of #866 but doesn't fix it completely.

@codecov
Copy link

codecov bot commented Dec 14, 2021

Codecov Report

Merging #867 (3e5f28d) into develop (78a8ef2) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
server/ladder_service.py 97.95% <100.00%> (-0.39%) ⬇️
server/lobbyconnection.py 94.30% <100.00%> (+0.05%) ⬆️
server/timing/timer.py 78.26% <0.00%> (-1.45%) ⬇️

@Askaholic Askaholic force-pushed the issue/#866-matchmaker-timeout branch from 606d75b to 3e5f28d Compare December 19, 2021 00:01
@Sheikah45
Copy link
Member

Game_id works

self.session = int(random.randrange(0, 4294967295))
self.protocol: Protocol = None
Copy link
Collaborator

@norraxx norraxx Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like which way typing is going! 👍

@@ -303,7 +303,7 @@ async def test_game_matchmaking_disconnect(lobby_server):

msg = await read_until_command(proto2, "match_cancelled", timeout=120)

assert msg == {"command": "match_cancelled"}
assert msg == {"command": "match_cancelled", "game_id": 41956}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this id set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be 1 higher than the highest game id in the test data.

@Askaholic Askaholic merged commit e5bfb9b into FAForever:develop Jan 2, 2022
@Askaholic Askaholic deleted the issue/#866-matchmaker-timeout branch January 2, 2022 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants